[RFC] Arches model QuerySets #11595 #11596
Conversation
jacobtylerwalls
requested review from
apeters,
robgaston,
philtweir,
chiatt,
bferguso and
chrabyrd
| def find_root_node(prefetched_siblings, nodegroup_id): | ||
| for sibling_node in prefetched_siblings: | ||
| if sibling_node.pk == nodegroup_id: | ||
| return sibling_node |
There was a problem hiding this comment.
It just occurred to me could add a column is_root_node to the Node model, perhaps as a GeneratedField so that we don't have to run these searches.
arches/app/models/querysets.py
Outdated
| if tile_val is not NOT_PROVIDED: | ||
| datatype_instance = datatype_factory.get_instance(node.datatype) | ||
| python_val = datatype_instance.to_python(tile_val) | ||
| setattr(tile, node.alias, python_val) |
There was a problem hiding this comment.
This is pretty well hidden (I'll declutter and add some signposting), but here is where I'm attaching the tile value to the tile object by node alias. Similar place in the ResourceInstanceQuerySet.
If we need to "seal" these python values so that we can know when they've been "tampered" with to make computation of saving back a little faster, I'm imagining we can wrap this in some sort of class with custom __setattr__ and __getattr__ methods to set that dirty state.
There was a problem hiding this comment.
Er, probably just on the __getattr__, and treat anything accessed as potentially dirty, because you can directly mutate things without setting something on the python object directly (unless we consider that a feature, that such an update is blocked...)
jacobtylerwalls
changed the title
jacobtylerwalls
force-pushed
the
jtw/pythonic-resource-models
branch
from
08b75b1 to
6273b5d
Compare
jacobtylerwalls
force-pushed
the
jtw/pythonic-resource-models
branch
from
236bbab to
15595f2
Compare
jacobtylerwalls
force-pushed
the
jtw/pythonic-resource-models
branch
from
8046ca4 to
bca5cc7
Compare
|
Sorry for the delay in getting a look through - looks like an interesting approach @jacobtylerwalls ! I do think this should be two bits of functionality though - the feedback from core team has been consistently that any such new functionality should be Applications, and I really think that should be the case here -- there are some general tidy-ups to the internals, which I think are great and are of general interest, but (and I acknowledge that we have an alternative approach, which is not relevant for everyone) tightly coupling the internals to DRF seems like a backward step. Some benefits of an Application: you would be able to progress development independently of core, and the release cycle as a new component could be much quicker. And, of course, it avoids expanding the volume of code to maintain internally in core, as well as keeping flexibility for other approaches - similarly to how we did the permissions groundwork, we were keen not to couple our model internally, (and hugely appreciate all the work Cyrus and Aaron did to make that a real thing in Arches 7.6!) |
|
Just to reiterate though, I think the concept looks really cool and it would be a massive step forward to have a way of doing RESTful model-based interaction, so kudos! |
Types of changes
Description of Change
This first iteration has TODOs enumerated and will need some tests, but this is functional enough to invite some testing and feedback. See #11595 for motivation. I'll update the bottom of this PR with DX questions that come up as I polish.
The basic idea:
annotate()feature for joining tile data to resource instance queriesTesting instructions
Issues Solved
Closes #11595
Checklist
Ticket Background
Further comments
TODO: flesh out DX questions below